Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: all dependencies of root package contribute to global hash #8202

Conversation

chris-olszewski
Copy link
Member

Description

With this PR we will now factor in all root dependency changes. Not just external packages.

Internal packages are handled by hashing all of the files contained in the package directory that aren't gitignore'd.

This does have performance implications as we can end up globwalking these directories multiple times and hashing the files multiple times if they end up as task inputs. This will be addressed in a future PR.

Testing Instructions

Added integration test that displays new behavior

Copy link

vercel bot commented May 23, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
examples-basic-web 🔄 Building (Inspect) Visit Preview 💬 Add feedback May 23, 2024 3:46pm
examples-native-web 🔄 Building (Inspect) Visit Preview 💬 Add feedback May 23, 2024 3:46pm
examples-nonmonorepo ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 23, 2024 3:46pm
examples-vite-web 🔄 Building (Inspect) Visit Preview 💬 Add feedback May 23, 2024 3:46pm
6 Ignored Deployments
Name Status Preview Comments Updated (UTC)
examples-designsystem-docs ⬜️ Ignored (Inspect) Visit Preview May 23, 2024 3:46pm
examples-gatsby-web ⬜️ Ignored (Inspect) Visit Preview May 23, 2024 3:46pm
examples-kitchensink-blog ⬜️ Ignored (Inspect) Visit Preview May 23, 2024 3:46pm
examples-svelte-web ⬜️ Ignored (Inspect) Visit Preview May 23, 2024 3:46pm
examples-tailwind-web ⬜️ Ignored (Inspect) Visit Preview May 23, 2024 3:46pm
rust-docs ⬜️ Ignored (Inspect) Visit Preview May 23, 2024 3:46pm

@chris-olszewski chris-olszewski marked this pull request as ready for review May 23, 2024 02:35
@chris-olszewski chris-olszewski requested a review from a team as a code owner May 23, 2024 02:35
Copy link
Contributor

github-actions bot commented May 23, 2024

🟢 CI successful 🟢

Thanks

Copy link
Contributor

github-actions bot commented May 23, 2024

🟢 Turbopack Benchmark CI successful 🟢

Thanks

"dependencies": {
"util": "*"
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't remember if our integration test fixtures generally use packageManager or not, but that might be useful since I know * has some weird behavior in different npm versions, especially with workspaces.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't remember if our integration test fixtures generally use packageManager or not, but that might be useful since I know * has some weird behavior in different npm versions, especially with workspaces

Good callout. With package manager requirements we now force usage of corepack for all test fixture defaulting to npm if one isn't specified in the fixture. Will open a ticket to make this explicit

@@ -17,7 +17,7 @@ Check
Tasks to Run
build
Task = build\s* (re)
Hash = 4047a6e65d7dafef
Hash = 81a933c332d3f388
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are all the hashes changing because we're including some kind of "none" value even when there are no root internal deps?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, since we're changing the Capnproto structure definition the hashing of it is shifting even when there aren't any internal deps.

@chris-olszewski chris-olszewski merged commit 0985a26 into turborepo_2 May 23, 2024
60 of 61 checks passed
@chris-olszewski chris-olszewski deleted the chrisolszewski/turbo-2709-root-package-internal-dependencies-treated-same-as-external branch May 23, 2024 17:50
chris-olszewski added a commit that referenced this pull request May 28, 2024
### Description

With this PR we will now factor in all root dependency changes. Not just
external packages.

Internal packages are handled by hashing all of the files contained in
the package directory that aren't gitignore'd.

This does have performance implications as we can end up globwalking
these directories multiple times and hashing the files multiple times if
they end up as task inputs. This will be addressed in a future PR.

### Testing Instructions

Added integration test that displays new behavior
chris-olszewski added a commit that referenced this pull request May 29, 2024
### Description

With this PR we will now factor in all root dependency changes. Not just
external packages.

Internal packages are handled by hashing all of the files contained in
the package directory that aren't gitignore'd.

This does have performance implications as we can end up globwalking
these directories multiple times and hashing the files multiple times if
they end up as task inputs. This will be addressed in a future PR.

### Testing Instructions

Added integration test that displays new behavior
chris-olszewski added a commit that referenced this pull request May 31, 2024
### Description

With this PR we will now factor in all root dependency changes. Not just
external packages.

Internal packages are handled by hashing all of the files contained in
the package directory that aren't gitignore'd.

This does have performance implications as we can end up globwalking
these directories multiple times and hashing the files multiple times if
they end up as task inputs. This will be addressed in a future PR.

### Testing Instructions

Added integration test that displays new behavior
chris-olszewski added a commit that referenced this pull request Jun 4, 2024
### Description

With this PR we will now factor in all root dependency changes. Not just
external packages.

Internal packages are handled by hashing all of the files contained in
the package directory that aren't gitignore'd.

This does have performance implications as we can end up globwalking
these directories multiple times and hashing the files multiple times if
they end up as task inputs. This will be addressed in a future PR.

### Testing Instructions

Added integration test that displays new behavior
@dipunm
Copy link

dipunm commented Aug 23, 2024

Hi all, can someone help me understand this change better? I came here from https://turbo.build/repo/docs/crafting-your-repository/upgrading#update-turbo-run-commands

I tested the following things:

  • I modified .md files in the .changeset folder in workspace root.
  • I changed ts and sh files in a build-scripts folder in workspace root.
  • Made change to an .md file at the root of the project.
  • Made change to .npmrc file at the root of the project.
  • Added a js file to the workspace root.

Each time, I ran turbo lint test and the cache was not cleared. Is this expected or not?

@mehulkar
Copy link
Contributor

mehulkar commented Sep 5, 2024

@dipunm I would open a discussion referencing this. Comments on merged/closed PRs are probably not going to get a response

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants